Skip to content

Go: migrate control flow graph to shared CFG library #2 - #22182

Open
owen-mc wants to merge 51 commits into
github:mainfrom
owen-mc:go/shared-cfg
Open

Go: migrate control flow graph to shared CFG library #2#22182
owen-mc wants to merge 51 commits into
github:mainfrom
owen-mc:go/shared-cfg

Conversation

@owen-mc

@owen-mc owen-mc commented Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR migrates the Go control-flow graph (CFG) from its bespoke, Go-specific implementation to the shared CFG library. Broadly speaking, the commits are in these groups:

  • The first commit changes the toString and getLocation predicates for many CFG classes and accepts all test changes.
  • Then there are a series of commits doing preliminary work such as adjusting tests or queries.
  • Core migration — Switch Go over to the shared CFG library and reimplement the language-specific pieces (the IR / node kinds) on top of it. The test changes are included in this commit, but there aren't too many because of the preceding commits.
  • A commit adding a print-CFG query and a commit adding CFG consistency queries, including their outputs.
  • IR clean-up — A sequence of small, focused simplifications that remove redundancy in the generated CFG nodes now that we're on the shared framework, e.g. unifying incdec-rhs/compound-rhs, folding zero-init and write nodes together, merging result-init into result-zero-init, dropping implicit slice-bound nodes, and no longer emitting CFG nodes for subexpressions of constant expressions.

I've tried to always make it so that a commit contains any test changes which it causes, so their effect can easily be seen while reviewing.

Note that additionalNodes is quite a lot bigger than in other languages, like java and C#. The reason for the disparity is architectural: Go's dataflow nodes are CFG-instruction-based (MkInstructionNode), not AST-based like Java/C#'s TExprNode. Switching to AST-keyed expr nodes (or mapping values via injects) would make Go's CFG implementation more like java and C#, but that's a larger IR redesign we'd do separately.

Copilot AI review requested due to automatic review settings July 13, 2026 16:39
@owen-mc
owen-mc requested review from a team as code owners July 13, 2026 16:39
@github-actions github-actions Bot added the Go label Jul 13, 2026
@owen-mc owen-mc mentioned this pull request Jul 13, 2026
Closed
github-advanced-security[bot]
github-advanced-security Bot found potential problems Jul 13, 2026
View reviewed changes

@github-advanced-security github-advanced-security AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Copilot AI reviewed Jul 13, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates Go control-flow, SSA, and data-flow modeling to the shared CFG framework, including schema support and updated test baselines.

Changes:

  • Integrates shared CFG/basic-block infrastructure and no-return modeling.
  • Adds synthesized range-element nodes with upgrade/downgrade support.
  • Updates framework models, inline annotations, and generated expectations.
Show a summary per file
File Description
go/ql/test/query-tests/Security/CWE-918/RequestForgery.expected Updates data-flow node labels.
go/ql/test/query-tests/Security/CWE-918/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/query-tests/Security/CWE-770/UncontrolledAllocationSize.expected Updates extraction-node labels.
go/ql/test/query-tests/Security/CWE-770/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/query-tests/Security/CWE-643/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/Security/CWE-640/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/Security/CWE-601/OpenUrlRedirect/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/query-tests/Security/CWE-601/BadRedirectCheck/main.go Relocates source annotations.
go/ql/test/query-tests/Security/CWE-601/BadRedirectCheck/cves.go Relocates source annotation.
go/ql/test/query-tests/Security/CWE-601/BadRedirectCheck/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/Security/CWE-601/BadRedirectCheck/BadRedirectCheck.go Relocates source annotation.
go/ql/test/query-tests/Security/CWE-347/MissingJwtSignatureCheck.expected Updates SSA locations and labels.
go/ql/test/query-tests/Security/CWE-347/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/Security/CWE-327/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/Security/CWE-326/InsufficientKeySize.expected Updates result ordering and SSA locations.
go/ql/test/query-tests/Security/CWE-322/InsecureHostKeyCallback.expected Updates SSA and extraction nodes.
go/ql/test/query-tests/Security/CWE-312/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/query-tests/Security/CWE-295/DisabledCertificateCheck/DisabledCertificateCheck.expected Updates assignment and literal labels.
go/ql/test/query-tests/Security/CWE-190/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/Security/CWE-117/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/Security/CWE-089/StringBreak.expected Updates extraction-node labels.
go/ql/test/query-tests/Security/CWE-089/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/query-tests/Security/CWE-089/CONSISTENCY/CfgConsistency.expected Adds CFG consistency baseline.
go/ql/test/query-tests/Security/CWE-079/StoredXss.expected Updates extraction and SSA nodes.
go/ql/test/query-tests/Security/CWE-079/stored.go Relocates source annotation.
go/ql/test/query-tests/Security/CWE-079/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/query-tests/Security/CWE-078/StoredCommand.expected Updates extraction-node labels.
go/ql/test/query-tests/Security/CWE-078/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/Security/CWE-078/CommandInjection.expected Updates SSA source ranges.
go/ql/test/query-tests/Security/CWE-022/UnsafeUnzipSymlink.expected Updates SSA locations.
go/ql/test/query-tests/Security/CWE-022/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/Security/CWE-020/MissingRegexpAnchor/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegexp/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/query-tests/RedundantCode/UnreachableStatement/CONSISTENCY/CfgConsistency.expected Adds CFG consistency baseline.
go/ql/test/query-tests/RedundantCode/RedundantRecover/tst.go Removes an obsolete alert annotation.
go/ql/test/query-tests/RedundantCode/RedundantRecover/RedundantRecover.expected Updates recover-call results.
go/ql/test/query-tests/RedundantCode/DeadStoreOfLocal/DeadStoreOfLocal.expected Updates assignment instruction labels.
go/ql/test/query-tests/RedundantCode/DeadStoreOfLocal/CONSISTENCY/CfgConsistency.expected Adds CFG consistency baseline.
go/ql/test/query-tests/RedundantCode/DeadStoreOfField/DeadStoreOfField.expected Updates assignment label.
go/ql/test/query-tests/InconsistentCode/UnhandledCloseWritableHandle/CONSISTENCY/CfgConsistency.expected Adds defer CFG baseline.
go/ql/test/query-tests/InconsistentCode/MissingErrorCheck/MissingErrorCheck.expected Updates SSA locations.
go/ql/test/library-tests/semmle/go/Types/notype.ql Excludes synthesized valid types.
go/ql/test/library-tests/semmle/go/security/SafeUrlFlow/SafeUrlFlow.expected Updates dereference labels.
go/ql/test/library-tests/semmle/go/Scopes/EntityWrite.expected Updates parameter-init nodes.
go/ql/test/library-tests/semmle/go/PrintAst/PrintAstExcludeComments.expected Adds range-element AST nodes.
go/ql/test/library-tests/semmle/go/PrintAst/PrintAst.expected Adds range-element AST nodes.
go/ql/test/library-tests/semmle/go/PrintAst/CONSISTENCY/CfgConsistency.expected Adds CFG consistency baseline.
go/ql/test/library-tests/semmle/go/IR/test.expected Updates extraction instruction labels.
go/ql/test/library-tests/semmle/go/frameworks/Yaml/yaml.go Updates inline model expectations.
go/ql/test/library-tests/semmle/go/frameworks/XNetHtml/SqlInjection.expected Updates extraction-node labels.
go/ql/test/library-tests/semmle/go/frameworks/XNetHtml/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/library-tests/semmle/go/frameworks/WebSocket/RemoteFlowSources.expected Updates extraction-node labels.
go/ql/test/library-tests/semmle/go/frameworks/WebSocket/Read.expected Updates extraction-node labels.
go/ql/test/library-tests/semmle/go/frameworks/WebSocket/CONSISTENCY/DataFlowConsistency.expected Updates dereference label.
go/ql/test/library-tests/semmle/go/frameworks/Twirp/server/main.go Relocates handler/source annotations.
go/ql/test/library-tests/semmle/go/frameworks/TaintSteps/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/library-tests/semmle/go/frameworks/SystemCommandExecutors/CONSISTENCY/DataFlowConsistency.expected Updates dereference label.
go/ql/test/library-tests/semmle/go/frameworks/Revel/test.expected Records inline expectation mismatch.
go/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go Relocates response-body annotation.
go/ql/test/library-tests/semmle/go/frameworks/Revel/OpenRedirect.expected Updates dereference labels.
go/ql/test/library-tests/semmle/go/frameworks/Protobuf/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/library-tests/semmle/go/frameworks/Protobuf/CONSISTENCY/CfgConsistency.expected Adds CFG consistency baseline.
go/ql/test/library-tests/semmle/go/frameworks/gqlgen/graph/schema.resolvers.go Relocates resolver annotation.
go/ql/test/library-tests/semmle/go/frameworks/GoMicro/main.go Relocates request annotation.
go/ql/test/library-tests/semmle/go/frameworks/GoMicro/LogInjection.expected Updates parameter SSA location.
go/ql/test/library-tests/semmle/go/frameworks/GoMicro/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/library-tests/semmle/go/frameworks/GoKit/main.go Relocates endpoint annotations.
go/ql/test/library-tests/semmle/go/frameworks/Gin/Gin.expected Updates extraction-node labels.
go/ql/test/library-tests/semmle/go/frameworks/Gin/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/fasthttp.go Updates inline source expectations.
go/ql/test/library-tests/semmle/go/frameworks/Fasthttp/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/library-tests/semmle/go/frameworks/Echo/CONSISTENCY/DataFlowConsistency.expected Updates dereference label.
go/ql/test/library-tests/semmle/go/frameworks/Chi/CONSISTENCY/DataFlowConsistency.expected Updates dereference label.
go/ql/test/library-tests/semmle/go/frameworks/Beego/test.go Relocates source annotation.
go/ql/test/library-tests/semmle/go/frameworks/Beego/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/library-tests/semmle/go/frameworks/Afero/CONSISTENCY/DataFlowConsistency.expected Updates dereference label.
go/ql/test/library-tests/semmle/go/dataflow/VarArgs/CONSISTENCY/DataFlowConsistency.expected Removes resolved consistency failure.
go/ql/test/library-tests/semmle/go/dataflow/ThreatModels/CONSISTENCY/DataFlowConsistency.expected Updates dereference label.
go/ql/test/library-tests/semmle/go/dataflow/SSA/VarUses.expected Updates result-read nodes.
go/ql/test/library-tests/semmle/go/dataflow/SliceExpressions/CONSISTENCY/CfgConsistency.expected Adds CFG consistency baseline.
go/ql/test/library-tests/semmle/go/dataflow/ReadsAndWrites/writesField.expected Updates field-write instructions.
go/ql/test/library-tests/semmle/go/dataflow/ReadsAndWrites/writesElement.expected Updates element-write instructions.
go/ql/test/library-tests/semmle/go/dataflow/ReadsAndWrites/readsMethod.expected Updates implicit dereference label.
go/ql/test/library-tests/semmle/go/dataflow/ReadsAndWrites/readsField.expected Updates implicit dereference label.
go/ql/test/library-tests/semmle/go/dataflow/ReadsAndWrites/readsElement.expected Updates implicit dereference label.
go/ql/test/library-tests/semmle/go/dataflow/PostUpdateNodes/test.expected Updates post-update dereference labels.
go/ql/test/library-tests/semmle/go/dataflow/Nodes/resultParameters.go Relocates result-node annotations.
go/ql/test/library-tests/semmle/go/dataflow/Nodes/ResultNode.expected Updates result-read nodes.
go/ql/test/library-tests/semmle/go/dataflow/Nodes/CallNode_getResult_int.expected Updates extraction-node labels.
go/ql/test/library-tests/semmle/go/dataflow/Nodes/BinaryOperationNodes.expected Updates compound-assignment label.
go/ql/test/library-tests/semmle/go/dataflow/HiddenNodes/test.expected Updates extraction-node labels.
go/ql/test/library-tests/semmle/go/dataflow/GlobalValueNumbering/GlobalValueNumber.expected Updates CFG instruction locations.
go/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionOutput_isResult_int.expected Updates result extraction labels.
go/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionOutput_getExitNode.expected Updates output exit nodes.
go/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionOutput_getEntryNode.expected Updates zero-init nodes.
go/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionInput_getExitNode.expected Updates parameter-init nodes.
go/ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionInput_getEntryNode.expected Updates SSA source ranges.
go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/steps.expected Updates extraction-node labels.
go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/srcs.expected Updates source-node locations.
go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/steps.expected Updates extraction-node labels.
go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/srcs.expected Updates source-node locations.
go/ql/test/library-tests/semmle/go/dataflow/DefaultTaintSanitizer/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/NoretFunctions.expected Updates normal-return classification.
go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/CONSISTENCY/CfgConsistency.expected Adds CFG consistency baseline.
go/ql/test/library-tests/semmle/go/concepts/Regexp/RegexpPattern.expected Updates extraction and SSA nodes.
go/ql/test/library-tests/semmle/go/concepts/HTTP/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/experimental/Unsafe/WrongUsageOfUnsafe.expected Updates SSA location.
go/ql/test/experimental/Unsafe/CONSISTENCY/CfgConsistency.expected Adds CFG consistency baseline.
go/ql/test/experimental/InconsistentCode/CONSISTENCY/CfgConsistency.expected Adds defer-loop CFG baseline.
go/ql/test/experimental/frameworks/CleverGo/CONSISTENCY/DataFlowConsistency.expected Updates dereference label.
go/ql/test/experimental/CWE-942/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/experimental/CWE-918/SSRF.expected Updates dereference and extraction labels.
go/ql/test/experimental/CWE-918/CONSISTENCY/DataFlowConsistency.expected Updates consistency output.
go/ql/test/experimental/CWE-840/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/experimental/CWE-807/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/experimental/CWE-74/DsnInjectionLocal.expected Updates dereference labels.
go/ql/test/experimental/CWE-369/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/experimental/CWE-321-V2/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/experimental/CWE-287/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/experimental/CWE-285/PamAuthBypass.expected Updates extraction-node label.
go/ql/test/experimental/CWE-203/CONSISTENCY/DataFlowConsistency.expected Updates dereference labels.
go/ql/test/example-tests/snippets/varwrite.expected Updates assignment label.
go/ql/test/example-tests/snippets/typeinfo.expected Updates parameter-init nodes.
go/ql/test/example-tests/snippets/fieldwrite.expected Updates assignment label.
go/ql/src/RedundantCode/UnreachableStatement.ql Reworks unreachable-statement detection.
go/ql/src/RedundantCode/DeadStoreOfLocal.ql Uses shared parameter initialization.
go/ql/src/experimental/IntegerOverflow/RangeAnalysis.qll Adapts increment/decrement analysis.
go/ql/lib/upgrades/b1341734d6870b105e5c9d168ce7dec25d7f72d0/upgrade.properties Declares range-element schema upgrade.
go/ql/lib/semmle/go/StringOps.qll Handles omitted slice lower bounds.
go/ql/lib/semmle/go/Stmt.qll Adds the range-element AST API.
go/ql/lib/semmle/go/Scopes.qll Adds no-normal-return modeling hook.
go/ql/lib/semmle/go/PrintAst.qll Makes the local overlay optional.
go/ql/lib/semmle/go/frameworks/Zap.qll Migrates no-return model.
go/ql/lib/semmle/go/frameworks/stdlib/Os.qll Migrates os.Exit model.
go/ql/lib/semmle/go/frameworks/stdlib/Log.qll Migrates fatal-log model.
go/ql/lib/semmle/go/frameworks/Revel.qll Adapts implicit field-read traversal.
go/ql/lib/semmle/go/frameworks/Logrus.qll Migrates fatal/panic models.
go/ql/lib/semmle/go/frameworks/Glog.qll Migrates fatal/exit models.
go/ql/lib/semmle/go/Expr.qll Synthesizes key-value expression types.
go/ql/lib/semmle/go/dataflow/SsaImpl.qll Connects SSA to the shared CFG.
go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll Refines switch-edge filtering.
go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll Adapts reachable and compound nodes.
go/ql/lib/semmle/go/dataflow/GlobalValueNumbering.qll Anchors side-effect lookup to CFG entry.
go/ql/lib/semmle/go/controlflow/BasicBlocks.qll Replaces bespoke basic blocks.
go/ql/lib/semmle/go/Concepts.qll Migrates heuristic fatal logging model.
go/ql/lib/printCfg.ql Adds the Go CFG viewer query.
go/ql/lib/go.dbscheme Adds @rangeelementexpr.
go/ql/consistency-queries/CfgConsistency.ql Enables shared CFG consistency checks.
go/extractor/extractor.go Extracts synthesized range-element nodes.
go/extractor/dbscheme/tables.go Registers the new expression kind.
go/downgrades/23f2f56b5d3a846b4f73e3fa62510e36f934fb46/upgrade.properties Configures downgrade transforms.
go/downgrades/23f2f56b5d3a846b4f73e3fa62510e36f934fb46/has_location.ql Removes synthesized-node locations.
go/downgrades/23f2f56b5d3a846b4f73e3fa62510e36f934fb46/exprs.ql Reparents range variables on downgrade.

Review details

  • Files reviewed: 191/192 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment thread go/ql/test/library-tests/semmle/go/frameworks/Revel/Revel.go Outdated
Comment thread go/ql/test/library-tests/semmle/go/dataflow/Nodes/resultParameters.go Outdated
Comment thread go/ql/test/library-tests/semmle/go/dataflow/Nodes/resultParameters.go Outdated
@owen-mc
owen-mc force-pushed the go/shared-cfg branch 3 times, most recently from d220d5c to ff0384e Compare July 16, 2026 06:46
@owen-mc
owen-mc requested a review from a team as a code owner August 14, 2026 13:50
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll
Comment on lines +279 to +283
// Go nests each case clause's body statements under the clause rather
// than in a flat list, so we expose a flattened view in which every
// case clause is immediately followed by its own body statements. This
// lets the shared library compute the body of a case as the statements
// between it and the next clause.

@aschackmull aschackmull Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. The shared lib actually supports both AST setups, but it does expect just a single body AstNode when the case bodies are nested under the case clauses. But since Go appears to have a sequence of statements as the body of a case clause, then I guess this is the easiest.

@owen-mc owen-mc Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we could extract a block statement to make it fit in with the shared CFG library better. But the workaround in ql is not too bad.

aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread shared/controlflow/codeql/controlflow/ControlFlowGraph.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread shared/controlflow/codeql/controlflow/ControlFlowGraph.qll Outdated
aschackmull
aschackmull reviewed Aug 18, 2026
View reviewed changes
Comment thread shared/controlflow/codeql/controlflow/ControlFlowGraph.qll
owen-mc and others added 23 commits September 4, 2026 09:49
@owen-mc
Address review comments
816261a
@owen-mc
Add defer CFG edge-case tests
a988fc9
@owen-mc
Build defer edges from an early Go CFG
832a96a
@owen-mc
Model Go function epilogues in the body CFG
13cfa68
@owen-mc
Address more review comments
e7e565b
@owen-mc
Change CompoundAssignStmt and IncDecStmt
21548fe
@owen-mc
Move getExpr to SwitchExpr
2a7ba81
@owen-mc
Remove/reduce comments
66c53f2
@owen-mc
Rename file to ControlFlowGraphImpl.qll
fad07af
@owen-mc
Use american spelling
f531531
@owen-mc
Add missing qldoc
655c2cd
@owen-mc
Update change note
65608bc
@owen-mc
Fix non-US spelling
503573b
@owen-mc
Change function epilogue CFG
3394940 
Use the pre-existing CFG library hooks more and removes some that we had added before.
@owen-mc
Remove CFG hook by making CFG for SelectStmt simpler
36f2b90
@owen-mc
Remove CFG hook preservesDefaultControlFlow
3e82736
@owen-mc
Allow empty switch to be a simple leaf node
43dfa1a
@owen-mc
Rename ForeachStmt to ForEachStmt
95c8541
@owen-mc
Undo unnecessary code changes
2e175ba
@owen-mc
Go: skip extracting ast.ParenExpr nodes
6345ef9 
Instead of creating a database entry for parenthesised expressions,
extract their child expression directly in their place. This makes the
extracted AST act as if ParenExpr nodes do not exist while still
correctly extracting children.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@owen-mc
Go: deprecate ParenExpr and remove all references
31dd8c1 
Since the extractor no longer produces ParenExpr nodes, deprecate
the class so existing user code gets a warning rather than breaking,
and remove all references from library code where it was used to
look through parentheses.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@owen-mc
Accept invalidAbruptCompletionOrigin results
f298ef0 
This is a new CFG consistency query.
@owen-mc
Fix invalidAbruptCompletionOrigin results
035cbfd
@owen-mc

owen-mc commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

There some changes to data flow consistency outputs, but they seem to be all changes of node label.

CFG consistency query changes which indicated a problem:

  • ParenExpr lacking an In node. I think the best solution for this is to stop extracting them. It's quick and easy and what we want to do at some point. No point trying to fix the CFG for them if they're going away soon anyway. I've done this.
  • The new query invalidAbruptCompletionOrigin highlighted that the abrupt completion for deferred calls panicking was starting on the DeferStmt rather than the CallExpr. I've fixed this.

CFG consistency query changes which don't indicate a problem:

  • select has multiple successors. Correct nondeterministic fan-out.
  • Empty select {} has no successor. Correct CFG: an empty select blocks forever.
  • Exit may invoke different/no deferred calls. Correct path-sensitive defer approximation.
  • Repeated defer registration produces invocation self-loop. Correct behaviour.

@aschackmull

aschackmull commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

CFG consistency query changes which don't indicate a problem:

  • select has multiple successors. Correct nondeterministic fan-out.
  • Empty select {} has no successor. Correct CFG: an empty select blocks forever.
  • Exit may invoke different/no deferred calls. Correct path-sensitive defer approximation.
  • Repeated defer registration produces invocation self-loop. Correct behaviour.

Makes sense, thanks. Although I'm slightly baffled by the existence of a block-forever language construct - that seems so pointless.

@aschackmull

aschackmull commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Alright, I think I'm generally happy with the changes now. Let's see what dca says.

@owen-mc
Go: restore data flow through pointer assignment targets
c5ffdee 
Evaluate the operand of a skipped pointer LHS and create post-update nodes for the full access path.
@owen-mc

owen-mc commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Looking into the one lost row of data flow consistency test results identified a bug in the new CFG. I've pushed an extra commit to fix it.

DCA shows some performance problems, which I'll investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aschackmull aschackmull aschackmull left review comments
Copilot code review Copilot
Copilot review effort, defaults to Balanced
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
+1 more reviewer
@github-advanced-security github-advanced-security[bot] github-advanced-security[bot] left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

documentation Go

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@owen-mc @aschackmull @github-advanced-security